Skip to content

chore: promote staging to main (2026-03-18 16:22 UTC)#1359

Merged
henrypark133 merged 1 commit intomainfrom
staging-promote/428303af-23255149035
Mar 18, 2026
Merged

chore: promote staging to main (2026-03-18 16:22 UTC)#1359
henrypark133 merged 1 commit intomainfrom
staging-promote/428303af-23255149035

Conversation

@ironclaw-ci
Copy link
Contributor

@ironclaw-ci ironclaw-ci bot commented Mar 18, 2026 •
edited by github-actions bot
Loading

Auto-promotion from staging CI

Batch range: 2784cef4d797cc8a36791010829c178b768a32b1..428303af1128e7f124ad623fc1338393a4d06fcc
Promotion branch: staging-promote/428303af-23255149035
Base: main
Triggered by: Staging CI batch at 2026-03-18 16:22 UTC

Commits in this batch (1):

Current commits in this promotion (1)

Current base: main
Current head: staging-promote/428303af-23255149035
Current range: origin/main..origin/staging-promote/428303af-23255149035

Auto-updated by staging promotion metadata workflow

Waiting for gates:

  • Tests: pending
  • E2E: pending
  • Claude Code review: pending (will post comments on this PR)

Auto-created by staging-ci workflow

@henrypark133
Redesign routine create requests for LLMs (#1147)
428303a 
* Redesign routine create requests for LLMs

* Fix panic-check false positives in routine tests

* Tighten routine schema requirements

* Tighten routine schema tests

* Mark test assertions safe for CI scan

* Align test assertions with panic scan

* Polish routine schema metadata

* Simplify routine test assertions

* Improve tool discovery guidance

* Clarify lightweight routine delivery prompts

* Fix routine delivery target defaults
@github-actions github-actions bot added scope: agent Agent core (agent loop, router, scheduler) scope: tool Tool infrastructure scope: tool/builtin Built-in tools scope: docs Documentation size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Mar 18, 2026
@claude
Copy link

claude bot commented Mar 18, 2026

Code review

Found 20 issues:

  1. [CRITICAL:100] N+1 schema generation on every LLM call — Tool::schema() in src/tools/builtin/routine.rs calls discovery_schema() which allocates new JSON for the complex routine_create schema (16 compatibility aliases + nested structures) on EVERY tool_definitions invocation. Schema should be cached or lazy-initialized.

ironclaw/src/tools/builtin/routine.rs

Lines 370 to 375 in 428303a

"timezone": "UTC"
},
"delivery": {
"channel": "telegram",
"user": "ops-team"
}

  1. [HIGH:100] Unbounded JSON mutation in routine_create_schema() — Loop at lines 476-601 mutates schema with 16 separate .insert() calls when building compatibility aliases. Creates transient allocations and repeated object lookups. Should batch allocations or use builder pattern.

ironclaw/src/tools/builtin/routine.rs

Lines 476 to 520 in 428303a

if include_compatibility_aliases {
if let Some(properties) = schema.get_mut("properties").and_then(Value::as_object_mut) {
properties.insert(
"trigger_type".to_string(),
serde_json::json!({
"type": "string",
"enum": ["cron", "event", "system_event", "manual"],
"description": "Compatibility alias for request.kind. Prefer request.kind."
}),
);
properties.insert(
"schedule".to_string(),
serde_json::json!({
"type": "string",
"description": "Compatibility alias for request.schedule. Prefer request.schedule."
}),
);
properties.insert(
"timezone".to_string(),
serde_json::json!({
"type": "string",
"description": "Compatibility alias for request.timezone. Prefer request.timezone."
}),
);
properties.insert(
"event_pattern".to_string(),
serde_json::json!({
"type": "string",
"description": "Compatibility alias for request.pattern when request.kind='message_event'."
}),
);
properties.insert(
"event_channel".to_string(),
serde_json::json!({
"type": "string",
"description": "Compatibility alias for request.channel when request.kind='message_event'."
}),
);
properties.insert(
"event_source".to_string(),
serde_json::json!({
"type": "string",
"description": "Compatibility alias for request.source when request.kind='system_event'."
}),
);

  1. [HIGH:100] Missing event_source in required fields for event_emit schema — event_emit_schema(false) returns schema with only ["event_type"] in required array, but parse_event_emit_args() requires both "event_source" and "event_type". Schema should have "required": ["event_source", "event_type"] from start.

ironclaw/src/tools/builtin/routine.rs

Lines 1095 to 1110 in 428303a

trigger,
action,
guardrails: RoutineGuardrails {
cooldown: Duration::from_secs(normalized.cooldown_secs),
max_concurrent: 1,
dedup_window: None,
},
notify: NotifyConfig {
channel: normalized.delivery.channel.clone(),
user: normalized.delivery.user.clone(),
..NotifyConfig::default()
},
last_run_at: None,
next_fire_at: next_fire,
run_count: 0,
consecutive_failures: 0,

  1. [HIGH:75] Prompt injection via unescaped channel/user in lightweight routines — build_lightweight_prompt() directly interpolates notify.channel and notify.user values into LLM prompt via format!() without escaping. Backticks provide no security boundary. Attacker controlling routine config could inject arbitrary prompt instructions.

ironclaw/src/agent/routine_engine.rs

Lines 853 to 863 in 428303a

if let Some(channel) = notify.channel.as_deref() {
full_prompt.push_str(&format!(
"The configured delivery channel for this routine is `{channel}`.\n"
));
}
if let Some(user) = notify.user.as_deref() {
full_prompt.push_str(&format!(
"The configured delivery target for this routine is `{user}`.\n"
));
}

  1. [HIGH:75] Suppressible clippy::too_many_arguments without builder pattern — RoutineEngine::new() accepts 8 parameters with #[allow(clippy::too_many_arguments)]. Violates extensible-design principle from CLAUDE.md. Builder or context struct would improve testability and reduce coupling.

ironclaw/src/agent/routine_engine.rs

Lines 68 to 76 in 428303a

config: RoutineConfig,
store: Arc<dyn Database>,
llm: Arc<dyn LlmProvider>,
workspace: Arc<Workspace>,
notify_tx: mpsc::Sender<OutgoingResponse>,
scheduler: Option<Arc<Scheduler>>,
tools: Arc<ToolRegistry>,
safety: Arc<SafetyLayer>,
) -> Self {

  1. [HIGH:75] JobContext created with Default pattern bypassing type safety — Line 1004 uses ..Default::default() leaving undocumented fields (channel, metadata, context_size) uninitialized. Fragile if defaults change. Should explicitly construct all fields or use factory function.

ironclaw/src/agent/routine_engine.rs

Lines 998 to 1008 in 428303a

let run_id = Uuid::new_v4();
let job_ctx = JobContext {
job_id: run_id,
user_id: routine.user_id.clone(),
title: "Lightweight Routine".to_string(),
description: routine.name.clone(),
..Default::default()
};
loop {
iteration += 1;

  1. [HIGH:75] Potential schema inconsistency for routine_create grouped format — When include_compatibility_aliases=false, schema properties for "request" don't have "required": ["kind"] at top level—only in oneOf variants. Could allow invalid requests to pass schema validation.

ironclaw/src/tools/builtin/routine.rs

Lines 450 to 475 in 428303a

"required": ["kind"]
})
},
"execution": if include_compatibility_aliases {
execution_discovery_schema()
} else {
serde_json::json!({
"type": "object",
"description": "Optional execution settings. Omit for the default lightweight mode.",
"properties": execution_properties()
})
},
"delivery": {
"type": "object",
"description": "Optional delivery defaults for notifications and message tool calls inside routine jobs.",
"properties": delivery_properties()
},
"advanced": {
"type": "object",
"description": "Optional advanced knobs. Most routines can omit this block.",
"properties": advanced_properties()
}
},
"required": ["name", "prompt"]
});

  1. [HIGH:95] Regex compilation on cache refresh without async bounds — refresh_event_cache() synchronously compiles regexes inside async loop. Regex compilation is CPU-bound and blocks async runtime with many event routines. No timeout or async cancellation.

ironclaw/src/agent/routine_engine.rs

Lines 92 to 134 in 428303a

pub async fn refresh_event_cache(&self) {
match self.store.list_event_routines().await {
Ok(routines) => {
let mut cache = Vec::new();
for routine in routines {
match &routine.trigger {
Trigger::Event { pattern, .. } => {
// Use RegexBuilder with size limit to prevent ReDoS
// from user-supplied patterns (issue #825).
match regex::RegexBuilder::new(pattern)
.size_limit(64 * 1024) // 64KB compiled size limit
.build()
{
Ok(re) => cache.push(EventMatcher::Message {
routine: routine.clone(),
regex: re,
}),
Err(e) => {
tracing::warn!(
routine = %routine.name,
"Invalid or too complex event regex '{}': {}",
pattern, e
);
}
}
}
Trigger::SystemEvent { .. } => {
cache.push(EventMatcher::System {
routine: routine.clone(),
});
}
_ => {}
}
}
let count = cache.len();
*self.event_cache.write().await = cache;
tracing::trace!("Refreshed event cache: {} routines", count);
}
Err(e) => {
tracing::error!("Failed to refresh event cache: {}", e);
}
}
}

  1. [HIGH:85] Large Vec allocation on every trigger check — Lines 145-151 and 235-241 allocate new Vec<Uuid> to collect routine IDs before batch query, even when cache is empty. Called on every message in agent loop.

ironclaw/src/agent/routine_engine.rs

Lines 145 to 155 in 428303a

let routine_ids: Vec<Uuid> = cache
.iter()
.filter_map(|matcher| match matcher {
EventMatcher::Message { routine, .. } => Some(routine.id),
EventMatcher::System { .. } => None,
})
.collect();
if routine_ids.is_empty() {
return 0;
}

  1. [MEDIUM:90] Excessive .clone() calls in data transformation — build_routine_trigger() and related functions clone entire Trigger/Action/NotifyConfig structs with nested String and HashMap clones. Happens on routine creation and event_emit calls. Should use references where possible.

ironclaw/src/agent/routine_engine.rs

Lines 913 to 940 in 428303a

.with_max_tokens(effective_max_tokens)
.with_temperature(0.3);
let response = ctx
.llm
.complete(request)
.await
.map_err(|e| RoutineError::LlmFailed {
reason: e.to_string(),
})?;
handle_text_response(
&response.content,
response.finish_reason,
response.input_tokens,
response.output_tokens,
)
}
/// Handle a text-only LLM response in lightweight routine execution.
///
/// Checks for the ROUTINE_OK sentinel, validates content, and returns appropriate status.
fn handle_text_response(
content: &str,
finish_reason: FinishReason,
total_input_tokens: u32,
total_output_tokens: u32,
) -> Result<(RunStatus, Option<String>, Option<i32>), RoutineError> {

  1. [MEDIUM:80] Discovery schema regeneration on every tool_info call — tool_info.rs:134 calls tool.discovery_schema() which rebuilds entire complex routine_create schema structure on every invocation. Should cache or lazy-initialize.

ironclaw/src/tools/builtin/tool_info.rs

Lines 130 to 140 in 428303a

let tool = registry.get(name).await.ok_or_else(|| {
ToolError::InvalidParameters(format!("No tool named '{name}' is registered"))
})?;
let schema = tool.discovery_schema();
let param_names = schema_param_names(&schema);
let mut info = serde_json::json!({
"name": tool.name(),
"description": tool.description(),
"parameters": param_names,

  1. [MEDIUM:75] Field extraction overhead in parsing — Helper functions string_field(), bool_field(), object_field() (lines 667-733) traverse nested objects with multiple .get() and .and_then() calls. Called ~20+ times per routine create request with redundant traversals.

ironclaw/src/tools/builtin/routine.rs

Lines 667 to 733 in 428303a

fn string_field(params: &Value, group: &str, field: &str, aliases: &[&str]) -> Option<String> {
nested_object(params, group)
.and_then(|obj| obj.get(field))
.and_then(Value::as_str)
.map(String::from)
.or_else(|| {
aliases
.iter()
.find_map(|alias| params.get(*alias).and_then(Value::as_str).map(String::from))
})
}
fn bool_field(params: &Value, group: &str, field: &str, aliases: &[&str]) -> Option<bool> {
nested_object(params, group)
.and_then(|obj| obj.get(field))
.and_then(Value::as_bool)
.or_else(|| {
aliases
.iter()
.find_map(|alias| params.get(*alias).and_then(Value::as_bool))
})
}
fn u64_field(params: &Value, group: &str, field: &str, aliases: &[&str]) -> Option<u64> {
nested_object(params, group)
.and_then(|obj| obj.get(field))
.and_then(Value::as_u64)
.or_else(|| {
aliases
.iter()
.find_map(|alias| params.get(*alias).and_then(Value::as_u64))
})
}
fn string_array_field(params: &Value, group: &str, field: &str, aliases: &[&str]) -> Vec<String> {
nested_object(params, group)
.and_then(|obj| obj.get(field))
.and_then(Value::as_array)
.or_else(|| {
aliases
.iter()
.find_map(|alias| params.get(*alias).and_then(Value::as_array))
})
.map(|arr| {
arr.iter()
.filter_map(|value| value.as_str().map(String::from))
.collect()
})
.unwrap_or_default()
}
fn object_field(
params: &Value,
group: &str,
field: &str,
aliases: &[&str],
) -> Option<Map<String, Value>> {
nested_object(params, group)
.and_then(|obj| obj.get(field))
.and_then(Value::as_object)
.cloned()
.or_else(|| {
aliases
.iter()
.find_map(|alias| params.get(*alias).and_then(Value::as_object).cloned())
})
}

  1. [MEDIUM:75] DRY violation: batch concurrent count queries duplicated — Lines 158-168 and 248-261 duplicate identical batch query pattern for counting concurrent routine runs. Extract to shared helper to prevent maintenance divergence.

ironclaw/src/agent/routine_engine.rs

Lines 158 to 170 in 428303a

let concurrent_counts = match self
.store
.count_running_routine_runs_batch(&routine_ids)
.await
{
Ok(counts) => counts,
Err(e) => {
tracing::error!("Failed to batch-load concurrent counts: {}", e);
return 0;
}
};
for matcher in cache.iter() {

  1. [MEDIUM:75] Circular dependency risk between RoutineEngine and tool registry — RoutineEngine constructed in agent_loop, then immediately register_routine_tools() passes engine to routine tools which hold Arc<RoutineEngine>, creating reference cycle with engine holding Arc<ToolRegistry>. Violates "generic/extensible architectures" principle.

ironclaw/src/agent/routine_engine.rs

Lines 68 to 76 in 428303a

config: RoutineConfig,
store: Arc<dyn Database>,
llm: Arc<dyn LlmProvider>,
workspace: Arc<Workspace>,
notify_tx: mpsc::Sender<OutgoingResponse>,
scheduler: Option<Arc<Scheduler>>,
tools: Arc<ToolRegistry>,
safety: Arc<SafetyLayer>,
) -> Self {

  1. [MEDIUM:70] Potential unbounded system_event filter processing — emit_system_event() iterates all cached system event routines and for EACH one iterates ALL filter keys in payload (lines 291-304) doing string comparisons. O(routines × filters) complexity.

https://github.com/nearai/ironclaw/blob/428303af1128e7f124ad623fc1338393a4d06fcc/src/agent/routine_engine.rs#T291-T304

  1. [MEDIUM:50] Missing validation of max_tool_rounds enforcement — max_tool_rounds capped at compile-time but schema allows arbitrary integers. If MAX_TOOL_ROUNDS_LIMIT > 5, schema promise is violated. Ensure schema max matches actual enforcement.

ironclaw/src/agent/routine_engine.rs

Lines 1040 to 1042 in 428303a

.tool_definitions_excluding(ROUTINE_TOOL_DENYLIST)
.await;

  1. [LOW:75] Notification emoji may fail on non-UTF8 systems — Lines 1283-1286 embed emoji (✅, 🔔, ❌, ⏳) directly in format strings. Per review-discipline.md UTF-8 rules, should be configurable or ASCII fallbacks for systems without emoji support.

ironclaw/src/agent/routine_engine.rs

Lines 1280 to 1290 in 428303a

}
let icon = match status {
RunStatus::Ok => "✅",
RunStatus::Attention => "🔔",
RunStatus::Failed => "❌",
RunStatus::Running => "⏳",
};
let message = match summary {
Some(s) => format!("{} *Routine '{}'*: {}\n\n{}", icon, routine_name, status, s),

  1. [LOW:60] Excessive string allocations in logging — truncate() function copies message content during routine firing. With frequent routine firing, allocations accumulate.

ironclaw/src/agent/routine_engine.rs

Lines 210 to 220 in 428303a

continue;
}
let detail = truncate(&message.content, 200);
self.spawn_fire(routine.clone(), "event", Some(detail));
fired += 1;
}
fired
}

  1. [LOW:50] Sentinel pattern "ROUTINE_OK" is fragile — Line 953 checks if content == "ROUTINE_OK" || content.contains("ROUTINE_OK"). The contains() check matches "ROUTINE_OK_FAILED" or "NOT_ROUTINE_OK". Use regex or stricter boundary checking.

ironclaw/src/agent/routine_engine.rs

Lines 950 to 960 in 428303a

}
// Check for the "nothing to do" sentinel
if content == "ROUTINE_OK" || content.contains("ROUTINE_OK") {
let total_tokens = Some((total_input_tokens + total_output_tokens) as i32);
return Ok((RunStatus::Ok, None, total_tokens));
}
let total_tokens = Some((total_input_tokens + total_output_tokens) as i32);
Ok((
RunStatus::Attention,

  1. [LOW:50] Regex pattern complexity not enforced on full_job context_paths — parse_routine_trigger() validates regex with 64KB limit but no similar validation for context_paths. While less exploitable, asymmetry suggests incomplete hardening of user-supplied patterns.

ironclaw/src/agent/routine_engine.rs

Lines 800 to 810 in 428303a

String::new()
}
};
// Determine max_tokens from model metadata with fallback
let effective_max_tokens = match ctx.llm.model_metadata().await {
Ok(meta) => {
let from_api = meta.context_length.map(|ctx| ctx / 2).unwrap_or(max_tokens);
from_api.max(max_tokens)
}
Err(_) => max_tokens,

This was referenced Mar 18, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@henrypark133 henrypark133 merged commit 59acab4 into main Mar 18, 2026
70 of 71 checks passed
@henrypark133 henrypark133 deleted the staging-promote/428303af-23255149035 branch March 18, 2026 21:16
henrypark133 added a commit that referenced this pull request Mar 18, 2026
@henrypark133 @claude
fix: address valid review comments from PR #1359
5edaebf 
- Cache discovery_schema() with OnceLock for routine tools (fixes #1361, #1371)
- Early-return on empty event cache before allocating Vec (fixes #1369)
- Extract batch concurrent count query helper to reduce duplication
- Fix ROUTINE_OK sentinel substring matching
- Migrate crate::safety import to ironclaw_safety per project convention

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@henrypark133 henrypark133 mentioned this pull request Mar 18, 2026
Merged
5 tasks
henrypark133 added a commit that referenced this pull request Mar 18, 2026
@henrypark133 @claude
fix: address valid review comments from PR #1359 (#1380)
ec04354 
- Cache discovery_schema() with OnceLock for routine tools (fixes #1361, #1371)
- Early-return on empty event cache before allocating Vec (fixes #1369)
- Extract batch concurrent count query helper to reduce duplication
- Fix ROUTINE_OK sentinel substring matching
- Migrate crate::safety import to ironclaw_safety per project convention

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 18, 2026
Open
Merged
Open
ilblackdragon pushed a commit that referenced this pull request Mar 19, 2026
@henrypark133 @claude @ilblackdragon
fix: address valid review comments from PR #1359 (#1380)
ecfab80 
- Cache discovery_schema() with OnceLock for routine tools (fixes #1361, #1371)
- Early-return on empty event cache before allocating Vec (fixes #1369)
- Extract batch concurrent count query helper to reduce duplication
- Fix ROUTINE_OK sentinel substring matching
- Migrate crate::safety import to ironclaw_safety per project convention

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: agent Agent core (agent loop, router, scheduler) scope: docs Documentation scope: tool/builtin Built-in tools scope: tool Tool infrastructure size: XL 500+ changed lines staging-promotion

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@henrypark133